Applying that fix resulted a completely red output (not to confuse this with the Dart error view).

Interesting. @jonasbark Is the red screen happening to all of your videos? Otherwise, would you mind sharing one of them for me to test? You can send them to ml @ rodrigo . red if your prefer. Cheers
For context, I've been running this patch on several iOS devices with no reported issues so far.

@recastrodiaz sorry - nevermind. My testing branch didn't yet receive the fix where sendInitialized was called too early.

No problem. Glad it works!

@recastrodiaz just for feedback purpose I've tested with this version below and the blackscreen problem seems to be solved.

video_player:
    git:
      url: https://github.com/recastrodiaz/plugins.git
      path: packages/video_player/video_player
      ref: 8344dcb20e2fe1884dba2745af1b0452df7fe163
    version: ^0.10.5+2

What version of flick_video_player can we use with this?

Same here, some videos play sound but video is black...

This fix works really great but it needs to be rebased on the current version. @recastrodiaz, do you need help to do so, if yes, how can I help you?
In our project we had to fork the 2.1.6 version and use this fix, but it would be better if it could be merged.

@letsar I don't mind rebasing this PR as this is a simple fix, I was doing it for a long time, but there seems to be no interest in merging it from the Flutter team so I'll avoid doing any work until this PR will be approved. This PR is more than 2y old after all...

Thanks @recastrodiaz, I understand your point of view. @cyanglaz can we do something about this PR? It's an annoying bug with what it seems to be an easy fix. Is there any blocker from the Flutter team about this?
From our side, we have to fork this plugin and apply this fix, but it's not really maintainable. It would be really great if we could merge this in the codebase so that anyone could profit from it.

Thanks @recastrodiaz, I understand your point of view. @cyanglaz can we do something about this PR?

This PR needs two things to move forward:

• Test coverage (as described in the contributing guide)
• Reviews (see below).

It's an annoying bug with what it seems to be an easy fix. Is there any blocker from the Flutter team about this?

The issue from a review standpoint is that it's not at all clear from the comments in the PR that the fix is correct, and that it won't regress other videos, because (unless I'm missing something) it doesn't seem that the root cause here is understood. If I'm reading it correctly it's saying that there exist some videos whose transforms are wrong, therefore all transforms of all videos will be replaced.

How widespread is the problem? Do we know why it happens in the first place? What effect does this have on videos whose transforms are correct? How many videos has this been tested on to understand what the overall effect will be? Are there examples of other widely used video players that do this, demonstrating that the approach is sound? Currently, it doesn't seem that there are answers to any of these questions, which would mean that a reviewer would need to spend a substantial amount of time trying to find those answers. As such, this is in the backlog and subject to our issue-based prioritization.

If someone can do the investigation above and update this PR with findings, that could certainly change the expected review effort such that it could be reviewed quickly.

@stuartmorgan thanks for your reply. Glad to hear this will be worked on.

Test coverage (as described in the contributing guide)
Reviews (see below).

I don't have much time ATM but feel free add tests as needed.

How widespread is the problem?

I'd say relatively widespread. If you don't control the videos being played, you'll eventually find one that does not render correctly (black screen).

Do we know why it happens in the first place?

I'd say this is a bug in AVFoundation, but I can't say for sure: https://developer.apple.com/documentation/avfoundation/avassettrack/1389837-preferredtransform
As usual, Apple's documentation is very sparse.

What effect does this have on videos whose transforms are correct?

None. This is a follow up bugfix that missed 1 case (180 degrees else if clause) in a PR that has already been merged. The other orientations are already using this same workaround, also authored by me: https://github.com/flutter/plugins/pull/690/files#diff-f4c7ef029bb5e0be8a6851b6b022c745dc4e99e3739f1c2f7f4944b85bc154a6R158

How many videos has this been tested on to understand what the overall effect will be?

I've been running my video_player fork in one of my apps for a couple of years and I have no reported black screen videos since then. User base is small of around 1K users a year ago.

Are there examples of other widely used video players that do this, demonstrating that the approach is sound?

Not that I am aware of.

What effect does this have on videos whose transforms are correct?

None. This is a follow up bugfix that missed 1 case (180 degrees else if clause) in a PR that has already been merged.

It's not just adding that case though, it's also making the entire block unconditional.

Hi,
is here any update? I really need to fix this black screen issue in my app.
Thanks.

@cyanglaz When will the bug fix be approved?

Please read the discussion above. The PR cannot be approved as-is.

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

What effect does this have on videos whose transforms are correct?

None. This is a follow up bugfix that missed 1 case (180 degrees else if clause) in a PR that has already been merged.

It's not just adding that case though, it's also making the entire block unconditional.

@recastrodiaz Can you address this question? It's still not at all clear to me why changing the behavior of the existing workaround to be applied in far more cases is necessary here.

@recastrodiaz Can you address this question? It's still not at all clear to me why changing the behavior of the existing workaround to be applied in far more cases is necessary here.

Absent a reply to this question, I don't see a way to move forward with this PR. Thank you for your contribution, and please don't hesitate to submit a new PR if you have the time to engage with a review. Thanks!

Hi @stuartmorgan, apologies for the late reply. This question fell of my radar.

My thinking at the time I made the initial commits is that the values of the Affine Transform needs to be exactly that of the video dimensions, any other values will render the video too big or too small.

I was unable to determine why sometimes some of the CGAffineTransform components are 0 and I couldn't find any sources that would explain this behaviour. IMO this a bug on Apple's side that was never fixed and the commits in this PR are just a work around that works. This is my best guess.

Nonetheless, the official video player still renders some videos black so it would be good if someone that has more time than me can take a look into fixing this.

I was unable to determine why sometimes some of the CGAffineTransform components are 0

I understand that. But I still don't understand why, if the goal of the PR is to fix an issue when the components are incorrectly zero, this PR removes the existing check that the workaround is only applied when the components are zero.